-
Notifications
You must be signed in to change notification settings - Fork 0
use urls from zos-config if exists #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
6831098
to
2bade3b
Compare
Signed-off-by: Ashraf Fouda <ashraf.m.fouda@gmail.com>
Signed-off-by: Ashraf Fouda <ashraf.m.fouda@gmail.com>
2d6d641
to
b41608f
Compare
5655d0d
to
13533ba
Compare
13533ba
to
87e059f
Compare
2683a49
to
67517d3
Compare
d476ac6
to
4c22507
Compare
eee4faa
to
69773c0
Compare
V4HubURL []string `json:"v4_hub_url"` | ||
|
||
// we should not be supporting flist url or hub storage from zos-config until we can update them on runtime | ||
// FlistURL string `json:"flist_url"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's have this flisturl configurable just in case we lost our urls again we can boot new machines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to verify that this will actually allow us to deploy a new machine and will not break the node entirely, will start working on that after verifying all changes done are working properly and well tested in all zos types, then will do that
env.GeoipURLs = geoip | ||
} | ||
|
||
// flist url and hub storage urls shouldn't listen to changes in config as long as we can't change it at run time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but if we faced samething when all *.tf were blocked then we will not be able to boot new machines and all those configurable urls will not help unless we can connect to the new storage url
} | ||
return hubStorage | ||
env := environment.MustGet() | ||
return env.HubStorage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should return the hubstorage based on if this is v3 or v4 here u r returning one hubstorage all the time
hubStorage = "zdb://hub.threefold.me:9900"
hubv4Storage = "zdb://hub.threefold.me:9940
they have diff port
even if u r doing that while creating the env but since u have this getter method make sure u use it every where and do ur logic here otherwise remove this func if not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
environment.Must get returns hubstorage with v3 if the node is of type v3 and v4 if the node is of type v4, so we don't need to do the check again here
@@ -246,7 +246,7 @@ func isLeastValidNode(ctx context.Context, farmID uint32, substrateGateway *stub | |||
// stop at three and quiet output | |||
err = exec.CommandContext(ctx, "ping", "-c", "3", "-q", ip).Run() | |||
if err != nil { | |||
log.Err(err).Msgf("failed to ping node %d", node.NodeID) | |||
log.Debug().Err(err).Msgf("failed to ping node %d", node.NodeID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did u change this to debug? this should be visible in the node logs that doesn't booted with debug flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it didn't have any log level. This error doesn't break the flow, the node logs it and tries to ping another node, so I thought it should be of level debug
Description
use config managed by environment.go from zos-config and use the ones defined in environment.go as default values
Related Issues
Checklist